Size Change: +34 B (0%)

Total Size: 1.11 MB

_{compressed-size-action}

Hey @iamthomasbishop 👋 this feature is ready to be reviewed, would you mind checking it out when you have some time?

• iOS build
• Android build

Thank you so much!

@geriux Thanks for the ping, this is looking and working great! I only have a few concerns, none of which are huge deals. I also noticed one semi-related thing, which isn't specific to this work but wanted to document in case we can tackle it soon. And then the last thing is a suggestion for future iterations, to have "on the record" 🙂 Let me know if you have any questions or if any of the requests will be especially challenging.

Contrast with button background

On the selected text color button, the icon can get lost in cases where the selected color is close to the background color of the button. Are we able to dynamically set the button background (behind the "A" icon) to the color of the actual block background behind it in the same way the web editor handles this? If not, we might want to consider a different treatment when a custom color is active.

(Note: I understand that this exact same problem will arise if the user manually sets the text color to something close to the background color, but at least in that case the colors are accurate to the output and represented on the canvas itself; as well as in the preview.)

Toolbar position

I'm curious, would it be difficult for us to move the colors button to a separate group on the block toolbar? As far as I know, we're simply following the order from the web, but I'm not sure if that is hard-coded or if we have flexibility in how we're organizing buttons/groups. If we have flexibility, I'd like to separate color out into its own group and place it either immediately after (preferred) or before the formatting group (bold, italic, etc), as demonstrated in these examples:

Semi-related

I'm not able to reproduce this on every tap so I'm unsure what the dimensions are on the "reset" tap target, but I noticed that tapping on or around the hex color label on the footer row of the top level sheet resets the value to its default. I'm assuming this is because most of our standard cell components act in a similar way, where the primary action's tap target spans the entire width of the cell. Ideally, the tap target for the "reset" button should only be as wide as the label (and perhaps slight bit of padding if there is any).

For future consideration

I noticed that if I have a custom color applied to a narrow selection — for example, a word or two in a sentence — then widen that selection to include the original (or go to the block-level color settings), then choose a new color for the wider selection, the previously-customized colors are essentially overridden. This might be expected behavior (I'm 100% sure it matches the behavior of web) but I can imagine a case where I'd like to "lock" certain color on part.

Hey there @antonis @iamthomasbishop 👋 this is ready for another review =)

Could you please check it when you have some time? These builds include the fixes to be able to set a text color when there's no text selected.

• WordPress iOS
• WordPress Android

Let me know if you find any issues while testing. Thank you!

Hey @antonis 👋

Thank you for the feedback!

I created new builds with a fix for the cases where the previous last character would change its color, can you please check it again and let me know?

There's one edge case though when you want to change the color right after a character that already has a color set. If you don't add a space to change into a new color, it doesn't work. This also happens in core, since this feature is more focused to "Highlight" a selected text.

@geriux I'm taking the latest build for a spin, it's looking great! I just have a couple of questions/bits of feedback, but I wouldn't necessarily consider them strictly blockers.

Non-block based theme initial state

For non-block-based themes — where we aren't able to display the site background-color and a block doesn't have an overridden background-color setting — can we set the default button background color on the ToolbarButton to the base surface color (white in light mode, black in dark mode) instead of the dark gray that the ToolbarButton currently is? I think this is the missing piece beyond the 3 edge cases you solved for in your previous round. Is something like this proposal possible?

With background, no text colors

Another thing, but this may be out of scope and may have already been discussed. When trying out different flows, I wondered if we should display the background color + default text color on the Toolbar in the case where I've first applied a custom BG color to a block but not adjusted any text colors. Example:

Various states

Here's a quick matrix of the various states, current vs. proposed:

For future consideration

I've also got some ideas for how we might refine the styling on the toolbar button itself to be a bit more flexible, but we can leave that for a future iteration. For the sake of future consideration, here is a quick sketch of what I'm imagining:

Hey @iamthomasbishop 👋

Thank you for the feedback! After our internal discussions in regards to the UI, I pushed the latest changes and you can check them once the builds are ready 🙇

@geriux Just tested the new build on iOS (iPhone 13 Pro and iPad Pro) and the changes we discussed yesterday are looking great. I'm not seeing any other issues on iOS but haven't had a chance to test on Android yet. I wouldn't expect any Android-specific issues, so if we can confirm that is the case then I think we can go ahead and ship this!

@geriux Just tested the new build on iOS (iPhone 13 Pro and iPad Pro) and the changes we discussed yesterday are looking great. I'm not seeing any other issues on iOS but haven't had a chance to test on Android yet. I wouldn't expect any Android-specific issues, so if we can confirm that is the case then I think we can go ahead and ship this!

Awesome!! 🚀 I checked the Android build and it was working as it was on iOS. Thank you so much for all the feedback and testing! 👏